Skip to content

Conversation

eggyal
Copy link
Contributor

@eggyal eggyal commented Dec 1, 2021

Addresses FIXME comment created in #78313

r? @lcnr

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 1, 2021
@rust-log-analyzer

This comment has been minimized.

@eggyal eggyal marked this pull request as draft December 1, 2021 18:19
@eggyal eggyal force-pushed the idfunctor-panic-safety branch from 403da27 to 51be4c7 Compare December 1, 2021 18:37
@rust-log-analyzer

This comment has been minimized.

@jackh726
Copy link
Member

jackh726 commented Dec 1, 2021

squints "free(): double free detected in tcache 2"

I think there's a bug here 😂

@eggyal
Copy link
Contributor Author

eggyal commented Dec 1, 2021

I think there's a bug here 😂

Aye, took me a while to spot it, but the assignment **slot = f(..)? was of course dropping the already-moved-out T-element.

@eggyal eggyal force-pushed the idfunctor-panic-safety branch from 51be4c7 to c471c6a Compare December 1, 2021 22:55
@eggyal eggyal marked this pull request as ready for review December 1, 2021 22:55
@bors
Copy link
Collaborator

bors commented Dec 2, 2021

☔ The latest upstream changes (presumably #91318) made this pull request unmergeable. Please resolve the merge conflicts.

@eggyal eggyal force-pushed the idfunctor-panic-safety branch from c471c6a to 026e155 Compare December 2, 2021 22:42
@lcnr
Copy link
Contributor

lcnr commented Dec 5, 2021

this does look right to me, but stuff like that's scary, so if someone else from @rust-lang/compiler-contributors wants to look at this, please do ✨

if not, i am going to approve this in a few days

@eggyal eggyal force-pushed the idfunctor-panic-safety branch from 026e155 to acd39ff Compare December 7, 2021 11:11
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 9, 2021
@lcnr
Copy link
Contributor

lcnr commented Dec 9, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 9, 2021

📌 Commit acd39ff has been approved by lcnr

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 9, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2021
Make IdFunctor::try_map_id panic-safe

Addresses FIXME comment created in rust-lang#78313

r? `@lcnr`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 10, 2021
Make IdFunctor::try_map_id panic-safe

Addresses FIXME comment created in rust-lang#78313

r? ``@lcnr``
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#91127 (Add `<*{const|mut} T>::{to|from}_bits`)
 - rust-lang#91310 (Add --out-dir flag for rustdoc)
 - rust-lang#91373 (Add needs-unwind to tests that depend on panicking)
 - rust-lang#91426 (Make IdFunctor::try_map_id panic-safe)
 - rust-lang#91515 (Add rsplit_array variants to slices and arrays)
 - rust-lang#91553 (socket ancillary data implementation for dragonflybsd.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7e18b79 into rust-lang:master Dec 11, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 11, 2021
@eggyal eggyal deleted the idfunctor-panic-safety branch December 12, 2021 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants